Skip to content

Fix internal error "Physical input schema should be the same as the one converted from logical input schema." #18412

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
Jefffrey merged 6 commits into from
Jan 11, 2026
Merged

Fix internal error "Physical input schema should be the same as the one converted from logical input schema." #18412

Jefffrey merged 6 commits into from
Jan 11, 2026

Conversation

@alamb
Copy link
Contributor

@alamb alamb commented Oct 31, 2025 •
edited
Loading

Which issue does this PR close?

Rationale for this change

It is a bug we are seeing in our production related to a schema mismatch

What changes are included in this PR?

  1. New slt test for the issue
  2. Properly compute the output field from window functions

Are these changes tested?

Yes

Are there any user-facing changes?

@github-actions github-actions bot added the sqllogictest SQL Logic Tests (.slt) label Oct 31, 2025
@alamb alamb mentioned this pull request Oct 31, 2025
Closed
@alamb alamb force-pushed the alamb/query_metadata_error branch from 1c7b1ca to 113eaa2 Compare January 6, 2026 19:01
@alamb alamb changed the title WIP: Fix internal error "Physical input schema should be the same as the one converted from logical input schema." Fix internal error "Physical input schema should be the same as the one converted from logical input schema." Jan 6, 2026
@github-actions github-actions bot added the logical-expr Logical plan and expressions label Jan 6, 2026
alamb
alamb commented Jan 6, 2026
View reviewed changes
datafusion/expr/src/expr_schema.rs
schema: &dyn ExprSchema,
window_function: &WindowFunction,
) -> Result<(DataType, bool)> {
) -> Result<FieldRef> {
Copy link
Contributor Author

@alamb alamb Jan 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the point is to return the entire field, rather than just the DataType and nullability so that any metadata attached to the field is also properly returned

I think the prior construction is from a time before Fields

@alamb alamb marked this pull request as ready for review January 6, 2026 20:00
crepererum
crepererum reviewed Jan 9, 2026
View reviewed changes
alamb
alamb commented Jan 9, 2026
View reviewed changes
@Jefffrey Jefffrey added this pull request to the merge queue Jan 11, 2026
Merged via the queue into apache:main with commit 9fa7500 Jan 11, 2026
32 checks passed
@Jefffrey
Copy link
Contributor

Jefffrey commented Jan 11, 2026

Thanks @alamb & @crepererum

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@crepererum crepererum crepererum approved these changes

@Jefffrey Jefffrey Jefffrey approved these changes

Assignees

No one assigned

Labels

logical-expr Logical plan and expressions sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Internal error: Physical input schema should be the same as the one converted from logical input schema.

3 participants

@alamb @Jefffrey @crepererum